Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to webview for previewing glTF files #123

Merged
merged 5 commits into from
Nov 8, 2018

Conversation

bghgary
Copy link
Contributor

@bghgary bghgary commented Nov 5, 2018

I have been working on a hackaton project to add a glTF debugger and I used the vscode extension as the foundation. This PR just does some clean up and switches the glTF preview to the vscode webview so that I can more easily pass messages to it. I have more changes coming.

I added the GLTF2.d.ts that I grabbed from the PR Don is submitting for DefinitelyTyped. We can switch over to the official one once that PR is merged.

@emackey
Copy link
Member

emackey commented Nov 5, 2018

Hi @bghgary, thanks so much for this PR! Do you mind getting approval to sign our CLA? Jamie Marconi and David Catuhe and a few other Microsoft folks have done this.

/cc #85

@bghgary
Copy link
Contributor Author

bghgary commented Nov 5, 2018

CLA sent.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2018

CLA received, thanks @bghgary!!

@emackey
Copy link
Member

emackey commented Nov 6, 2018

Thanks @bghgary. Currently can't compile. I suspect something is messed up on my side. Here's what I get:

src/gltfPreview.ts(13,51): error TS2694: Namespace ''vscode'' has no exported member 'WebviewPanel'.
src/gltfPreview.ts(44,35): error TS2339: Property 'createWebviewPanel' does not exist on type 'typeof window'.
src/gltfPreview.ts(74,47): error TS2694: Namespace ''vscode'' has no exported member 'WebviewPanel'.

Obviously these API points are supposed to exist. I tried npm upgrade and npm install. Feels like I've overlooked something.

@bghgary
Copy link
Contributor Author

bghgary commented Nov 6, 2018

Ahh, yes. Sorry, I missed adding a package.json change.

@bghgary
Copy link
Contributor Author

bghgary commented Nov 6, 2018

This should work now. I was separating the rest of my changes from this PR and missed a couple of things.

src/gltfPreview.ts Outdated Show resolved Hide resolved
@emackey
Copy link
Member

emackey commented Nov 7, 2018

If I manually specify a reflection environment via the settings, for either Babylon or ThreeJS, I get CORS errors in the console and the environment won't load.

VSCode User Settings file:

    "glTF.Babylon.environment": "D:\\emackey\\EnvMaps\\StonewallEnvHDR.dds",

Output:

Failed to load file:///D:/emackey/EnvMaps/StonewallEnvHDR.dds: Cross origin requests are only supported for protocol schemes: http, data, chrome, vscode-resource, chrome-extension, https.
retryLoop @ vscode-resource:c:/Users/emackey/Documents/Git/gltf-vscode/node_modules/babylonjs/babylon.max.js:9040

@emackey
Copy link
Member

emackey commented Nov 7, 2018

I like the persistence, you can cycle to other webviews and come back, and the page is still alive and doesn't need to reload. This could open the door to some interesting user interactions on the 3D view itself.

Fixes #85.

@bghgary
Copy link
Contributor Author

bghgary commented Nov 7, 2018

the page is still alive and doesn't need to reload

Yes, that's part of the reason why I'm doing this. I have code locally that interacts with the Babylon viewer. It's not quite yet, but I wanted to get the infrastructure merged in first.

@najadojo
Copy link
Contributor

najadojo commented Nov 7, 2018

Shouldn't be needed those two systems are similar enough.

@bghgary
Copy link
Contributor Author

bghgary commented Nov 7, 2018

If I manually specify a reflection environment via the settings, for either Babylon or ThreeJS, I get CORS errors in the console and the environment won't load.

Every webview panel gets created with a list of paths that it is allowed to access. I will probably make it such that the default environment's path is part of this list when the panel is created. The list of paths cannot be updated after the panel is created. This means that the default environment will not update until the preview window is closed and reopened. I think this is okay, but wanted to call it out.

@emackey
Copy link
Member

emackey commented Nov 7, 2018

This means that the default environment will not update until the preview window is closed and reopened.

I think that's fine. Later we could add a trigger to destroy and create a new webview when that setting changes, if that becomes important. But I don't change it that often, probably I'll just close and reopen the window manually. For this PR that's fine.

@bghgary
Copy link
Contributor Author

bghgary commented Nov 7, 2018

I've updated with the fix for the environment configuration.

Copy link
Member

@emackey emackey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bghgary, this looks great.

@emackey emackey merged commit ff466dd into AnalyticalGraphicsInc:master Nov 8, 2018
@emackey
Copy link
Member

emackey commented Nov 9, 2018

Version 2.1.17 published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants